Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.87%. Comparing base (e3d3849) to head (5498f26).

@@            Coverage Diff             @@
##              dev    #3686      +/-   ##
==========================================
+ Coverage   89.85%   89.87%   +0.01%     
==========================================
  Files         322      322              
  Lines       10380    10400      +20     
==========================================
+ Hits         9327     9347      +20     
  Misses       1053     1053              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Have identified an issue with the cover state not reliably changing to open/closed. Investigating this currently.

@TheJulianJES, @dmulcahey looking at the code in https://github.com/zigpy/zha/blob/dev/zha/application/platforms/cover/__init__.py
it looks to me like the _determine_state function and stored target positions might be the cause of the issue, I'm seeing similar issues reported for other devices which would appear to have the same root cause as below home-assistant/core#93836, home-assistant/core#98933.

I've tested the following scenarios:

1. go_to_lift_percentage has never been called, down_close is called, then the device reports position_or_tilt of 0
_target_lift_position == None
position_or_tilt == 0
returns STATE_CLOSED

2. go_to_lift_percentage has never been called, up_open is called, then the device reports position_or_tilt of 100
_target_lift_position == None
position_or_tilt == 100
returns: STATE_OPEN

3. go_to_lift_percentage is called (go to 0), then the device reports position_or_tilt of 0
_target_lift_position == 0
position_or_tilt == 0
returns STATE_CLOSED

4. go_to_lift_percentage is called (go to 100), then the device reports position_or_tilt of 100
_target_lift_position == 100
position_or_tilt == 100
returns STATE_OPEN

5. go_to_lift_percentage is called (go to 50), then the device reports position_or_tilt of 50
_target_lift_position == 50
position_or_tilt == 50
returns STATE_OPEN

6. go_to_lift_percentage has previously been called (stored value is 50), down_close is called, then the device reports position_or_tilt of 0
_target_lift_position == 50
position_or_tilt == 0
returns STATE_CLOSED

7. go_to_lift_percentage has previously been called (stored value is 50), up_open is called, then the device reports position_or_tilt of 100
_target_lift_position == 50
position_or_tilt == 100
returns None, state remains as STATE_OPENING

8. go_to_lift_percentage has previously been called (stored value is 50) and current position is 100, stop is called
_target_lift_position == 100 (the function async_stop_cover sets it to the current cover position)
position_or_tilt == 100
returns STATE_OPEN

I'd suggest we can handle this in one of 3 ways:

1. set the stored _target_lift_position to 100 or 0 when calling the up_open or down_close commands respectively
2. clear the stored _target_lift_position when calling the up_open or down_close commands
3. modify the _determine_state function to return STATE_OPEN when self.is_opening == True and position_or_tilt == 100 is true

Below is the current function for reference:

    def _determine_state(self, position_or_tilt, is_lift_update=True) -> None:
        """Determine the state of the cover.

        In HA None is unknown, 0 is closed, 100 is fully open.
        In ZCL 0 is fully open, 100 is fully closed.
        Keep in mind the values have already been flipped to match HA
        in the WindowCovering cluster handler
        """
        if is_lift_update:
            target = self._target_lift_position
            current = self.current_cover_position
        else:
            target = self._target_tilt_position
            current = self.current_cover_tilt_position

        if position_or_tilt == 0:
            self._state = (
                STATE_CLOSED
                if is_lift_update
                else STATE_OPEN
                if self.current_cover_position is not None
                and self.current_cover_position > 0
                else STATE_CLOSED
            )
            return
        if target is not None and target != current:
            # we are mid transition and shouldn't update the state
            return
        self._state = STATE_OPEN